Skip to content

Add extra check for competing proof + sort by nonce on ring buffer#7891

Open
sstanculeanu wants to merge 7 commits into
feat/testnet-fixesfrom
medium-findings
Open

Add extra check for competing proof + sort by nonce on ring buffer#7891
sstanculeanu wants to merge 7 commits into
feat/testnet-fixesfrom
medium-findings

Conversation

@sstanculeanu

Copy link
Copy Markdown
Collaborator

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@sstanculeanu sstanculeanu self-assigned this Jun 25, 2026
@sstanculeanu sstanculeanu added the type:bug Something isn't working label Jun 25, 2026
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.47%. Comparing base (f46eb92) to head (0fc3488).

Files with missing lines Patch % Lines
consensus/spos/bls/v2/subroundEndRound.go 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           feat/testnet-fixes    #7891      +/-   ##
======================================================
- Coverage               77.47%   77.47%   -0.01%     
======================================================
  Files                     887      887              
  Lines                  126452   126463      +11     
======================================================
+ Hits                    97963    97971       +8     
- Misses                  22043    22045       +2     
- Partials                 6446     6447       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens consensus behavior and sync heuristics by adding an extra guard when a competing proof exists, and by adjusting nonce sequence detection logic to be nonce-ordered rather than arrival-ordered. It also introduces/validates a new process config knob used by the sync logic and updates test defaults accordingly.

Changes:

  • Drop received signature shares during EndRound when a competing block proof is detected.
  • Sort the “recent desynced nonces” sample before checking consecutivity to better handle out-of-order proof arrivals.
  • Add RoundModulusTriggerWhenSyncIsStuck defaults + validation, and extend unit tests to cover the new validation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
testscommon/components/configs.go Adds the new round-level process config default used in tests/common setups.
consensus/spos/bls/v2/subroundEndRound.go Drops signatures early when a competing proof is present.
consensus/spos/bls/ntpsync/roundSyncController.go Sorts nonce samples before consecutivity check to make resync trigger independent of arrival order.
common/configs/processConfigs.go Validates RoundModulusTriggerWhenSyncIsStuck is non-zero to prevent modulo-by-zero.
common/configs/processConfigs_test.go Updates configs used in tests and adds a test for the new validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +95 to +99
@@ -95,6 +96,8 @@ func areNoncesInAscendingOrder(nonces []uint64) bool {
return false
}

slices.Sort(nonces)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines +140 to +142
if cfg.RoundModulusTriggerWhenSyncIsStuck == 0 {
return fmt.Errorf("%w for RoundModulusTriggerWhenSyncIsStuck", process.ErrInvalidValue)
}

@sstanculeanu sstanculeanu Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

ssd04
ssd04 previously approved these changes Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants